Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for parsing negative numbers in sexps #1655

Merged
merged 2 commits into from
Jul 24, 2023
Merged

Add support for parsing negative numbers in sexps #1655

merged 2 commits into from
Jul 24, 2023

Conversation

ddickstein
Copy link
Contributor

The motivation for this came from failing to parse a response that Merlin returned due to the -1 values for col in the start and end fields:

((assoc)
 (class . "return")
 (value (
   (assoc)
   (start (assoc) (line . 0) (col . -1))
   (end (assoc) (line . 0) (col . -1))
   (type . "typer")
   (sub)
   (valid . true)
   (message . "omitted")))
 (notifications)
 (timing (assoc) (clock . 166) (cpu . 145) (query . 1) (pp . 0) (reader . 21) (ppx . 36) (typer . 76) (error . 11)))

Add support for parsing negative numbers in sexps
4177e0a 
The motivation for this came from failing to parse a response that Merlin
returned due to the -1 values for col in the start and end fields:

((assoc)
 (class . "return")
 (value (
   (assoc)
   (start (assoc) (line . 0) (col . -1))
   (end (assoc) (line . 0) (col . -1))
   (type . "typer")
   (sub)
   (valid . true)
   (message . "omitted")))
 (notifications)
 (timing (assoc) (clock . 166) (cpu . 145) (query . 1) (pp . 0) (reader . 21) (ppx . 36) (typer . 76) (error . 11)))
@voodoos
Copy link
Collaborator

voodoos commented Jul 21, 2023

Hi @ddickstein, the patch looks okay, but I'm curious to know how the issue is triggered exactly.
The output you show is an error query result which contains dummy positions (as defined in Lexing.dummy_pos, for reference). What is you use-case for re-feeding part of this output to Merlin ?

Also, this change will require a changelog entry :-)

@ddickstein
Copy link
Contributor Author

The use case is that we pass the returned data from Merlin to a process that extracts and records telemetry information, but Merlin serializes sexps differently from sexplib (maps, in particular), so we need to use Merlin's logic to deserialize it.

@voodoos
Copy link
Collaborator

voodoos commented Jul 24, 2023

Ok, thanks for the bit of context. We could consider moving to an established sexp library at some point...

@voodoos voodoos merged commit 0065474 into ocaml:master Jul 24, 2023
voodoos added a commit to voodoos/merlin that referenced this pull request Aug 24, 2023
@voodoos
[B] Add support for parsing negative numbers in sexps (ocaml#1655)
d6ffeb6 
from ddickstein/parse-negative-numbers
voodoos added a commit to voodoos/opam-repository that referenced this pull request Aug 24, 2023
@voodoos
[new release] merlin, merlin-lib and dot-merlin-reader (4.10-414)
2bc5e35 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
voodoos added a commit to voodoos/opam-repository that referenced this pull request Aug 24, 2023
@voodoos
[new release] merlin, merlin-lib and dot-merlin-reader (4.10-500)
e56b3a0 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
voodoos added a commit to voodoos/opam-repository that referenced this pull request Aug 24, 2023
@voodoos
[new release] merlin, merlin-lib (4.10-414)
f445238 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
voodoos added a commit to voodoos/opam-repository that referenced this pull request Aug 24, 2023
@voodoos
[new release] merlin, merlin-lib (4.10-500)
38b91b3 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
@voodoos voodoos mentioned this pull request Aug 24, 2023
Merged
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
@voodoos @nberth
[new release] merlin, merlin-lib (4.10-414)
dccf2b8 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
@voodoos @nberth
[new release] merlin, merlin-lib (4.10-500)
bc02e79 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
Backport to 501
Status: 4.10-500
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ddickstein @voodoos